src/routes/chat.$chatId.lazy.tsx (1)

47-56: Type-safety issue: chatId union vs cast; also gate the query with enabled

• The component compares chatId !== "new" but chatId was cast to Id<"chats"> earlier. This loses the union type and weakens type safety.
• Align with other hooks by adding enabled: chatId !== "new" for clarity and to avoid unnecessary fetches.

  const {
    data: queryChat,
    isError: isChatError,
    error: chatError,
  } = useQuery({
-    ...convexQuery(
+    ...convexQuery(
       api.chats.queries.get,
       chatId !== "new" ? { chatId: chatId as Id<"chats"> } : "skip"
     ),
+    enabled: chatId !== "new",
  });

Additionally, update the chatId typing (outside the selected range) to preserve the union and satisfy the project’s strict Id typing guideline:

// At Line 38 (outside selected range)
const rawChatId = params.chatId as string;
const chatId: Id<"chats"> | "new" =
  rawChatId === "new" ? "new" : (rawChatId as Id<"chats">);

src/components/chat/panels/projects/document-list.tsx (1)

47-55: Early return hides loading & error states

While isLoadingProjectDocs is true (or the query errored), projectDocuments is undefined, so this block executes and the user sees “No documents added yet” instead of a spinner or error.

- if (!projectDocuments || projectDocuments.projectDocuments.length === 0) {
+ if (!isLoadingProjectDocs &&
+     !isProjectDocumentsError &&
+     (!projectDocuments || projectDocuments.projectDocuments.length === 0)) {

Move the loading / error checks above this guard or incorporate them into the condition so the correct UI is shown.

src/components/chat/input/document-list.tsx (1)

105-112: Duplicate / early return null hides spinner & error UI

The first null-return executes while data is still loading ( documents is undefined ), so the new loading/error components never render.
Remove the premature check and keep a single “no documents” guard after the loading/error blocks.

-  if (!documents?.length) return null;
...
-  if (!documents?.length) return null;

Place one “no documents” check after Line 134 instead.

src/components/chat/messages/index.tsx (1)

20-41: Missing deps ⇒ stale UI on error changes

useMemo omits isError, error, isStreamError, and streamError from its dependency list, so entering/leaving error states won’t trigger a re-render.

- }, [isLoading, isEmpty, streamStatus, chatId, userLoadable]);
+ }, [
+   isLoading,
+   isEmpty,
+   isError,
+   error,
+   isStreamError,
+   streamError,
+   streamStatus,
+   chatId,
+   userLoadable,
+ ]);

Also applies to: 113-116

src/components/app-sidebar/index.tsx (1)

232-237: “No matching chats” never shown

The guard !isSearching makes this branch skip when searching, so users get no feedback for an empty search result set.

- {displayHistoryChats.length === 0 && !isSearching && (
+ {displayHistoryChats.length === 0 && (

Then keep the conditional text selection (isSearching ? ...) as-is.

src/components/chat/messages/user-message.tsx (1)

283-292: Show utils bar on focus for keyboard users, not only on hover

Add a focus-visible path to avoid hiding controls from non-pointer users.

-        <div className="opacity-0 flex gap-2 group-hover/message:opacity-100 transition-opacity">
+        <div className="opacity-0 flex gap-2 group-hover/message:opacity-100 group-focus-within/message:opacity-100 transition-opacity">

src/hooks/chats/use-chats.ts (1)

149-150: isSearching semantics: clearer and less brittle

Using debouncedQuery.trim().length > 0 is a cleaner signal than comparing two strings. Good change.

If you want the UI to reflect “user is typing” immediately (before debounce triggers), consider also deriving a non-debounced flag for UX (optional).

src/components/ui/loading-spinner.tsx (1)

4-8: Make layout styling extensible without breaking current uses

Currently className applies to the icon. If callers need to style the wrapper, consider a non-breaking addition: containerClassName.

 interface LoadingSpinnerProps {
   sizeClassName?: string;
   className?: string;
   label?: string;
+  containerClassName?: string;
 }
 
 export function LoadingSpinner({
   sizeClassName = "h-5 w-5",
   className,
   label,
+  containerClassName,
 }: LoadingSpinnerProps) {
   return (
-    <div className="flex items-center justify-center gap-2">
+    <div className={cn("flex items-center justify-center gap-2", containerClassName)}>
       <Loader2
         aria-label="loading"
         className={cn(
           "animate-spin text-muted-foreground",
           sizeClassName,
           className
         )}
       />
-      <p className="text-muted-foreground">{label}</p>
+      {label ? <p className="text-muted-foreground">{label}</p> : null}
     </div>
   );
 }

Also applies to: 10-14

src/hooks/chats/use-documents.ts (1)

12-18: Avoid duplicated skip conditions (skip arg vs enabled)

You’re using both chatId !== "new" ? { chatId } : "skip" and enabled: chatId !== "new". One is sufficient. Using both is harmless but redundant and can introduce divergence if conditions drift.

Pick one approach (recommended: keep the skip sentinel only for consistency with other hooks in this PR) and drop the other.

src/routes/auth.tsx (1)

4-4: Consistent loading UI via LoadingSpinner

Good replacement; this standardizes the auth loading state with the rest of the app.

Optionally add a label for clarity/accessibility:

-        <LoadingSpinner sizeClassName="w-8 h-8" />
+        <LoadingSpinner sizeClassName="w-8 h-8" label="Checking authentication…" />

Also applies to: 20-20

src/components/chat/panels/projects/chat-list.tsx (2)

18-25: React Query state destructuring looks good

State exposure is clear. Note: React Query’s error is typed as unknown; ensure ErrorState can accept unknown or cast appropriately where passed.

---

27-33: Loading state handling is clear and accessible

Spinner + label is appropriate. Consider adding aria-busy on the container for a11y.

-  <div className="flex items-center justify-center py-10">
+  <div className="flex items-center justify-center py-10" aria-busy="true" aria-live="polite">

src/routes/chat.$chatId.lazy.tsx (1)

62-78: Good explicit error UI; consider adding loading handling

The error state is well-structured and consistent. Optionally add a loading spinner for the chat fetch to avoid brief content flashes while resolving the chat.

src/components/ui/accordion.tsx (1)

153-155: Avoid running rotation animation when icon is hidden

Currently the wrapper still animates even if the icon isn’t rendered. Minor optimization and avoids layout quirks by conditionally rendering the wrapper too.

-        <motion.div
-          animate={{ rotate: isOpen ? 0 : 180 }}
-          transition={{ duration: 0.25 }}
-          className="items-center flex justify-center "
-        >
-          {showIcon && (
-            <ChevronDownIcon className="text-muted-foreground pointer-events-none size-4" />
-          )}
-        </motion.div>
+        {showIcon && (
+          <motion.div
+            animate={{ rotate: isOpen ? 0 : 180 }}
+            transition={{ duration: 0.25 }}
+            className="items-center flex justify-center "
+          >
+            <ChevronDownIcon className="text-muted-foreground pointer-events-none size-4" />
+          </motion.div>
+        )}

src/components/chat/panels/projects/list.tsx (1)

44-53: Loading state UX is solid; add aria-busy for a11y

Minor accessibility enhancement.

-  <div className="flex flex-1 items-center justify-center">
+  <div className="flex flex-1 items-center justify-center" aria-busy="true" aria-live="polite">

src/routes/settings/profile.tsx (1)

40-48: Pass a non-undefined error to ErrorState

ErrorState receives error={userError}, but when isErrorUser is true and userError is still undefined, the component will not get any details.
Consider using error: userError ?? new Error("Unknown error") (or omit the prop) to avoid passing undefined.

src/components/chat/input/toolbar/index.tsx (1)

324-329: Include the underlying error for debuggability

ErrorState can show richer context if the error prop is supplied, mirroring other usages in the codebase.

- <ErrorState
+ <ErrorState
     density="compact"
     title="Failed to load project"
+    error={/* isProjectError object here */}
     className="h-9"
   />

src/components/topnav.tsx (1)

101-105: Spinner markup produces duplicate text

Inside a dropdown-sized area the <LoadingSpinner> already renders its own label; the sibling <span> repeats it.
Consider removing the extra <span> to avoid redundant content.

src/components/chat/input/toolbar/projects-dropdown.tsx (1)

59-112: Minor: inconsistency in loading UI

The dropdown renders both LoadingSpinner and a text node, but elsewhere in the codebase the spinner component already contains the label. Align with the rest of the app for consistency:

-              <DropdownMenuItem>
-                <LoadingSpinner
-                  className="h-4 w-4"
-                  label="Loading projects..."
-                />
-                <span className="text-muted-foreground">
-                  Loading projects...
-                </span>
-              </DropdownMenuItem>
+              <DropdownMenuItem>
+                <LoadingSpinner className="h-4 w-4" label="Loading projects..." />
+              </DropdownMenuItem>

src/components/document-dialog.tsx (3)

24-35: Redundant enabled + "skip" usage

enabled: !!documentDialogOpen already prevents the query from firing when the dialog is closed.
Returning "skip" in the query key is therefore unnecessary noise.

---

55-65: "file" handled twice

case "file" in this switch already fetches a download URL; the subsequent default‐branch check for document.type === "file" will never be reached.
Removing the duplicate simplifies control-flow and avoids an extra needless request if the tag ever mis-matches.

Also applies to: 75-83

---

180-189: Unreachable in-dialog error state

The component returns early for isDocumentError || documentError (lines 114-132), so this inner error block is never rendered.
Delete to avoid dead code and accidental divergence in future edits.

src/components/ui/error-state.tsx (1)

64-71: Early null-return can hide explicit titles

If the caller passes a custom title but neither error nor description, the early return null prevents the component from rendering despite valid props.

Consider dropping the guard or checking title separately.

src/components/chat/messages/user-message.tsx (5)

32-41: Good adoption of explicit loading/error states in DocumentButton; minor cleanups

Nicely aligns with the PR goal. Consider two small tweaks:

• Rely on isDocumentError (it implies error exists) instead of isDocumentError || documentError.
• Use consistent title-case microcopy.

-  if (isDocumentError || documentError) {
+  if (isDocumentError) {
     return (
       <div className="py-2 max-w-xs">
         <ErrorState
           density="compact"
-          title="error loading attached documents"
+          title="Error loading attached documents"
           error={documentError}
           showDescription={false}
         />
       </div>
     );
   }

Also applies to: 43-52, 54-65

---

43-52: Spinner copy/style consistency

Consider tightening copy to match other places and singular/plural cases.

-        <LoadingSpinner
-          sizeClassName="h-4 w-4"
-          label="Fetching attached documents"
-        />
+        <LoadingSpinner sizeClassName="h-4 w-4" label="Loading document…" />

---

67-74: Guard against missing doc name to avoid empty button

If the query returns no data or an unnamed doc, provide a safe fallback label.

-      {documentData?.name}
+      {documentData?.name ?? "View document"}

---

85-92: EditingDocumentList loading/error handling looks solid

The loading spinner and ErrorState simplify UX and match the PR’s consistency goal. Consider matching microcopy tone and adding a label for the loading state.

-      <div className="px-2 py-1 text-xs text-muted-foreground flex items-center gap-2">
-        <LoadingSpinner sizeClassName="h-3 w-3" /> Loading…
+      <div className="px-2 py-1 text-xs text-muted-foreground flex items-center gap-2">
+        <LoadingSpinner sizeClassName="h-3 w-3" label="Loading documents…" />
       </div>

Also applies to: 102-116

---

229-251: Optional: avoid N queries by batching document fetches

Rendering one useQuery per DocumentButton can fan out requests for messages with many attachments. Consider prefetching via a single getMultiple for all file_ids in content and pass names down (or render directly) to reduce request overhead and spinner churn.

Example approach:

• Extract all file_ids from content in a useMemo.
• useQuery({...convexQuery(api.documents.queries.getMultiple, { documentIds })}).
• Build a Map<Id<"documents">, Document> and render names inline, avoiding per-item queries.

Also applies to: 242-246

temp.md (4)

5-8: Fix bare URL and clarify phrasing

Format the URL and tighten the copy for readability.

-- update these
-  tool-streaming, because of vibz mcp as it one shots the generation. so we need to live stream to the user all the changes.
-  https://github.com/0bs-chat/zerobs/tree/feat/message-queue : the message queue function.
+- Update tool streaming. Vibz MCP currently one-shots generation, so we need to live-stream updates to the user.
+- <https://github.com/0bs-chat/zerobs/tree/feat/message-queue> — the message queue function.

---

9-11: Consistency and capitalization (OAuth, MCP, UI); improve readability

-- vibe coding (better auth -> convex cloud migration -> streaming tool calls -> convex oauth integration -> revamp mcp templates to pass along the env vars)
-  custom ui for vibz mcp. (like artifacts, we will replace the panel content with the ui for vibz)(preview, dashboard (convex dashboard), code (vs code))
+- Vibe coding (Better Auth → Convex Cloud migration → streaming tool calls → Convex OAuth integration → revamp MCP templates to pass along env vars).
+  Custom UI for Vibz MCP (like Artifacts): replace the panel content with the Vibz UI (Preview, Dashboard (Convex dashboard), Code (VS Code)).

---

12-13: Grammar and style nits (capitalize proper nouns; UX; “etc.” with period)

-- migrate to better auth (when i get the green light from mantra after better-auth integrates.)
+- Migrate to Better Auth (when I get the green light from Mantra after Better Auth integrates).
 ...
-- improve ux overall with loading states and whatnot.
+- Improve UX overall with loading states and related polish.
-- google integration (the code is already there just need to setup oauth)
+- Google integration (the code is already there; just need to set up OAuth).
-- business related mcp with ability to autofill connection info (like auto fetching api key/oauth key for the headers in mcp using oauth, etc to reduce friction)
+- Business‑related MCP with ability to autofill connection info (e.g., auto‑fetching API key/OAuth key for MCP request headers using OAuth, etc.) to reduce friction.

Also applies to: 15-15, 21-23

---

25-27: Clarify actionable items

-- need message queue system
-- immediate send, wait for file to be processed check on the frontend instead of backend
+- Implement message queue system.
+- Immediate send: perform “wait for file to be processed” check on the frontend instead of the backend.

src/components/chat/panels/projects/details.tsx (2)

52-61: Spinner sizing and a11y nit

Prefer sizeClassName for sizing (avoids conflicting Tailwind classes), and add status/a11y attributes to the wrapper.

-  if (isLoadingProject) {
-    return (
-      <div className="flex flex-1 items-center justify-center py-8">
-        <LoadingSpinner
-          className="h-6 w-6"
-          label="Loading project details..."
-        />
-      </div>
-    );
-  }
+  if (isLoadingProject) {
+    return (
+      <div
+        className="flex flex-1 items-center justify-center py-8"
+        role="status"
+        aria-busy="true"
+        aria-live="polite"
+      >
+        <LoadingSpinner
+          sizeClassName="h-6 w-6"
+          label="Loading project details..."
+        />
+      </div>
+    );
+  }

---

63-73: Tighten error condition and unify layout with loading

• Rely on isProjectError (React Query’s source of truth) and avoid the redundant || projectError.
• Match the loading view’s centered layout. The current className=" " looks accidental.

-  if (isProjectError || projectError) {
+  if (isProjectError) {
     return (
-      <div className=" ">
+      <div className="flex flex-1 items-center justify-center py-8">
         <ErrorState
           className="h-full py-2.5"
           title="Error loading project details"
           error={projectError}
         />
       </div>
     );
   }

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

Learnt from: CR
PR: 0bs-chat/zerobs#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-01T07:34:38.459Z
Learning: Applies to convex/**/*.{ts,js} : Try to use as few calls from actions to queries and mutations as possible to avoid race conditions.

Learnt from: CR
PR: 0bs-chat/zerobs#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-01T07:34:38.459Z
Learning: Applies to convex/**/*.{ts,js} : Use ctx.db.replace to fully replace an existing document. This method will throw an error if the document does not exist.

Learnt from: CR
PR: 0bs-chat/zerobs#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-01T07:34:38.459Z
Learning: Applies to convex/**/*.{ts,js} : Convex queries do NOT support .delete(). Instead, .collect() the results, iterate, and call ctx.db.delete(row._id) on each result.

Learnt from: CR
PR: 0bs-chat/zerobs#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-01T07:34:38.459Z
Learning: Applies to convex/**/*.{ts,js} : Use .unique() to get a single document from a query. This method will throw an error if there are multiple documents.

Learnt from: CR
PR: 0bs-chat/zerobs#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-01T07:34:38.459Z
Learning: Applies to convex/**/* : Convex uses file-based routing; organize files with public query, mutation, or action functions within the convex/ directory.

Learnt from: CR
PR: 0bs-chat/zerobs#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-01T07:34:38.459Z
Learning: Applies to convex/**/*.{ts,js} : Use ctx.runQuery to call a query, ctx.runMutation to call a mutation, and ctx.runAction to call an action. Only call an action from another action if you need to cross runtimes (e.g., from V8 to Node). Otherwise, pull out shared code into a helper async function.

Learnt from: CR
PR: 0bs-chat/zerobs#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-01T07:34:38.459Z
Learning: Applies to convex/**/*.{ts,js} : Use internalQuery, internalMutation, and internalAction to register internal functions (private, only callable by other Convex functions). Use query, mutation, and action to register public functions (exposed to the public Internet). Do NOT use query, mutation, or action for sensitive internal functions.

Learnt from: CR
PR: 0bs-chat/zerobs#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-01T07:34:38.459Z
Learning: Applies to convex/**/*.{ts,js} : ALWAYS use the new function syntax for Convex functions (using query/mutation/internalQuery/internalMutation/action/internalAction with args, returns, and handler).

Learnt from: CR
PR: 0bs-chat/zerobs#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-01T07:34:38.459Z
Learning: Applies to convex/**/*.{ts,js} : All ctx.runQuery, ctx.runMutation, and ctx.runAction calls take a FunctionReference. Do NOT pass the callee function directly.

Learnt from: CR
PR: 0bs-chat/zerobs#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-01T07:34:38.459Z
Learning: Applies to convex/**/*.{ts,js} : Use the api object from convex/_generated/api.ts to call public functions and the internal object to call internal functions.

Learnt from: CR
PR: 0bs-chat/zerobs#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-01T07:34:38.459Z
Learning: Applies to convex/schema.ts : Always import schema definition functions from convex/server in convex/schema.ts.

Learnt from: CR
PR: 0bs-chat/zerobs#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-01T07:34:38.459Z
Learning: Applies to convex/**/*.{ts,js} : Never use ctx.db inside of an action. Actions don't have access to the database.

Learnt from: CR
PR: 0bs-chat/zerobs#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-01T07:34:38.459Z
Learning: Applies to convex/**/*.{ts,js} : Always add "use node"; to the top of files containing actions that use Node.js built-in modules.

Learnt from: CR
PR: 0bs-chat/zerobs#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-01T07:34:38.459Z
Learning: Applies to convex/**/*.{ts,js} : Convex storage stores items as Blob objects. You must convert all items to/from a Blob when using Convex storage.

Learnt from: CR
PR: 0bs-chat/zerobs#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-01T07:34:38.459Z
Learning: Applies to convex/**/*.{ts,js} : When using ctx.runQuery, ctx.runMutation, or ctx.runAction to call a function in the same file, specify a type annotation on the return value to work around TypeScript circularity limitations.

Learnt from: CR
PR: 0bs-chat/zerobs#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-01T07:34:38.459Z
Learning: Applies to convex/**/*.{ts,js} : ALWAYS include argument and return validators for all Convex functions (query, internalQuery, mutation, internalMutation, action, internalAction). If a function doesn't return anything, include returns: v.null() as its output validator.

Learnt from: CR
PR: 0bs-chat/zerobs#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-01T07:34:38.459Z
Learning: Applies to convex/**/*.{ts,js} : When using async iteration, don't use .collect() or .take(n) on the result of a query. Use for await (const row of query) instead.

src/routes/settings/apiKeys.tsx (2)

245-258: Add error handling to toggle/remove mutations

These mutations currently assume success. Failures will silently do nothing beyond server logs, which contradicts the PR goal of improved error handling.

Wrap both in try/catch and surface failures via toasts:

   const handleToggle = async (config: ApiKeyConfig, enabled: boolean) => {
-    await updateApiKey({
-      key: config.key,
-      enabled,
-    });
-    toast.success(
-      `${config.title} ${enabled ? "enabled" : "disabled"} successfully`
-    );
+    try {
+      await updateApiKey({
+        key: config.key,
+        enabled,
+      });
+      toast.success(
+        `${config.title} ${enabled ? "enabled" : "disabled"} successfully`
+      );
+    } catch (error) {
+      toast.error(`Failed to ${enabled ? "enable" : "disable"} ${config.title}`);
+      console.error(error);
+    }
   };
 
   const handleRemove = async (config: ApiKeyConfig) => {
-    await removeApiKey({ key: config.key });
-    toast.success(`${config.title} removed successfully`);
+    try {
+      await removeApiKey({ key: config.key });
+      toast.success(`${config.title} removed successfully`);
+    } catch (error) {
+      toast.error(`Failed to remove ${config.title}`);
+      console.error(error);
+    }
   };

---

154-169: Align UI with the existing OPENAI_API_KEY

The backend and convex/langchain/models.ts exclusively use "OPENAI_API_KEY". To avoid user confusion and ensure the correct key is stored, update the UI copy and icon to reference OpenAI (Option A).

• Keep key: "OPENAI_API_KEY" unchanged.
• Update in src/routes/settings/apiKeys.tsx:

• Change title from "OpenRouter API Key" to "OpenAI API Key".
• Point the link at https://platform.openai.com/api-keys and link text to “OpenAI”.
• Swap the icon from <Icons.OpenRouter /> to <Icons.OpenAI />.

Suggested diff:

@@ src/routes/settings/apiKeys.tsx
-    title: "OpenRouter API Key",
+    title: "OpenAI API Key",
     description: (
       <>
-        Required for GPT models and AI-powered features. Get your API key from{" "}
-        <ExternalLink href="https://openrouter.ai/settings/keys">
-          OpenRouter
-        </ExternalLink>
+        Required for GPT models and AI-powered features. Get your API key from{" "}
+        <ExternalLink href="https://platform.openai.com/api-keys">
+          OpenAI
+        </ExternalLink>
       </>
     ),
-    icon: <Icons.OpenRouter />,
+    icon: <Icons.OpenAI />,

src/routes/__root.tsx (1)

105-111: Use the provided open value in onOpenChange to avoid stale state

The SidebarProvider’s onOpenChange callback supplies the next open state, so toggling via !sidebarOpen can capture a stale value. Update to pass the new value directly:

• File: src/routes/__root.tsx
Lines: 105–111

-          <SidebarProvider
-            className="…"
-            open={sidebarOpen}
-            onOpenChange={() => {
-              setSidebarOpen(!sidebarOpen);
-            }}
-          >
+          <SidebarProvider
+            className="…"
+            open={sidebarOpen}
+            onOpenChange={(open: boolean) => {
+              setSidebarOpen(open);
+            }}
+          >

src/routes/__root.tsx (1)

76-83: Resolved prior issue: state updates moved out of render

This addresses the earlier feedback about mutating state during render on settings routes. Thanks for fixing it.

src/routes/settings/apiKeys.tsx (3)

27-31: Lazy-load favicons and mark as decorative to avoid redundant screen reader output

These provider icons are purely decorative (the provider name is rendered right next to them). To improve a11y and slightly reduce network/paint cost, consider lazy-loading and using empty alt text (or aria-hidden). Optionally, suppress the Referer header for privacy with referrerPolicy.

Apply this diff to each img tag:

-      <img
-        src="https://www.google.com/s2/favicons?sz=256&domain_url=https%3A%2F%2Fopenai.com%2F"
-        alt="OpenAI"
-        className="w-4 h-4 dark:invert"
-      />
+      <img
+        src="https://www.google.com/s2/favicons?sz=256&domain_url=https%3A%2F%2Fopenai.com%2F"
+        alt=""
+        aria-hidden="true"
+        loading="lazy"
+        decoding="async"
+        referrerPolicy="no-referrer"
+        className="w-4 h-4 dark:invert"
+      />

Repeat similarly for Google, Exa, and OpenRouter icons (keeping provider-specific classes intact).

Also applies to: 36-40, 45-49, 54-58

---

280-285: Disable toggle when no key value is set

Prevent enabling a provider with no stored key to avoid backend errors and confusing UX.

             <Switch
               id={`toggle-${config.key}`}
               checked={isEnabled}
               onCheckedChange={(checked) => handleToggle(config, checked)}
+              disabled={!existingKey?.value}
             />

---

131-141: Ensure masked key truncates properly and doesn’t blow layout

The mask can be very long and may overflow. Add min-w-0 to the flex child and apply truncate to the value span for reliable ellipsis in flex layouts.

-    <div className="flex items-center justify-between p-3 bg-input/80 rounded-md">
-      <div className="flex items-center gap-1 truncate">
+    <div className="flex items-center justify-between p-3 bg-input/80 rounded-md">
+      <div className="flex items-center gap-1 min-w-0">
         <span className="text-sm font-medium">Current Value:</span>
-        <span className="text-sm font-mono">
+        <span className="text-sm font-mono truncate">
           {config.isPassword ? maskKey(existingKey.value) : existingKey.value}
         </span>
       </div>

src/routes/__root.tsx (1)

76-83: Small polish: tighten effect dependencies and calls

You can remove sidebarOpen from deps and set both states unconditionally when entering settings. This avoids an extra effect run caused by toggling sidebarOpen to false.

-  useEffect(() => {
-    if (!isSettingsRoute) return;
-    sidebarOpen && setSidebarOpen(false);
-    setResizePanelOpen(false);
-  }, [isSettingsRoute, sidebarOpen, setSidebarOpen, setResizePanelOpen]);
+  useEffect(() => {
+    if (!isSettingsRoute) return;
+    setSidebarOpen(false);
+    setResizePanelOpen(false);
+  }, [isSettingsRoute, setSidebarOpen, setResizePanelOpen]);

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

src/hooks/chats/use-documents.ts (1)

111-118: Potential crash if chat.documents is undefined

Spreading possibly-undefined chat.documents will throw. Default to [] to be safe.

-          await updateChatMutation({
-            chatId,
-            updates: {
-              documents: [...chat.documents, ...documentIds],
-            },
-          });
+          await updateChatMutation({
+            chatId,
+            updates: {
+              documents: [...(chat.documents ?? []), ...documentIds],
+            },
+          });

src/components/chat/input/document-list.tsx (1)

105-121: Fix: Loading/error branches are unreachable due to early return ordering

The early return on Line 105 exits before loading/error UI, so users never see the spinner or error state. Reorder checks and remove the duplicate empty check.

-  if (!documents?.length) return null;
-
-  const selectedModel = models.find((m) => m.model_name === model);
-  const modalities = selectedModel?.modalities;
-
-  if (!documents?.length) return null;
-
-  if (isLoadingDocuments) {
-    return (
-      <div className="flex items-center justify-start p-2">
-        <div className="text-sm text-muted-foreground flex items-center gap-2">
-          <LoadingSpinner sizeClassName="h-4 w-4" />
-          Loading attached documents...
-        </div>
-      </div>
-    );
-  }
-
-  if (isDocumentsError || documentsError) {
+  if (isLoadingDocuments) {
+    return (
+      <div className="flex items-center justify-start p-2">
+        <div className="text-sm text-muted-foreground flex items-center gap-2">
+          <LoadingSpinner sizeClassName="h-4 w-4" />
+          Loading attached documents...
+        </div>
+      </div>
+    );
+  }
+
+  if (isDocumentsError || documentsError) {
     return (
       <div className="flex items-center justify-start p-2">
         <ErrorState
           title="Error loading documents"
           showDescription={false}
           className="p-2"
           density="compact"
         />
       </div>
     );
   }
+
+  if (!documents?.length) return null;
+
+  const selectedModel = models.find((m) => m.model_name === model);
+  const modalities = selectedModel?.modalities;

src/components/chat/messages/user-message.tsx (1)

25-31: Type safety: use Id<'documents'> for fileId instead of string

Aligns with project guidelines and removes the need for unsafe casts.

-const DocumentButton = ({
-  fileId,
-  setDocumentDialogOpen,
-}: {
-  fileId: string;
-  setDocumentDialogOpen: (id: Id<"documents"> | undefined) => void;
-}) => {
+const DocumentButton = ({
+  fileId,
+  setDocumentDialogOpen,
+}: {
+  fileId: Id<"documents">;
+  setDocumentDialogOpen: (id: Id<"documents"> | undefined) => void;
+}) => {
@@
   } = useQuery({
     ...convexQuery(api.documents.queries.get, {
-      documentId: fileId as Id<"documents">,
+      documentId: fileId,
     }),
   });
@@
-    <Button
-      className="py-6 cursor-pointer"
-      onClick={() => setDocumentDialogOpen(fileId as Id<"documents">)}
-    >
+    <Button
+      className="py-6 cursor-pointer"
+      onClick={() => setDocumentDialogOpen(fileId)}
+    >

Also applies to: 38-41, 68-73

src/components/app-sidebar/index.tsx (1)

232-237: Fix empty-state condition so “No matching chats found” can show

The current guard !isSearching prevents the search empty-state from rendering. Let the conditional depend only on the list being empty and choose the label based on isSearching.

-                {displayHistoryChats.length === 0 && !isSearching && (
-                  <div className="text-sm text-muted-foreground text-center py-2">
-                    {isSearching
-                      ? "No matching chats found"
-                      : "No chat history"}
-                  </div>
-                )}
+                {displayHistoryChats.length === 0 && (
+                  <div className="text-sm text-muted-foreground text-center py-2">
+                    {isSearching ? "No matching chats found" : "No chat history"}
+                  </div>
+                )}

src/components/topnav.tsx (1)

44-46: Rules of Hooks violation: conditional return before hooks

Returning <Navigate /> conditionally means subsequent hooks (useEffect, useParams) aren’t called on that render. This breaks hook ordering between renders.

Remove the early return:

-  if (isErrorUser) {
-    return <Navigate to="/auth" />;
-  }

Add a side-effect redirect instead (place after hooks are declared), ensuring you don’t loop on the auth route and clearing stale user state:

useEffect(() => {
  if (!isErrorUser) return;

  // Optional: Only redirect on Unauthorized errors (see earlier comment)
  // if (!isUnauthorized(userError)) return;

  // Clear any stale user data before redirecting
  setUser(undefined);

  // Avoid redirect loops if TopNav also renders on the auth route
  if (location.pathname !== "/auth") {
    navigate({ to: "/auth" });
  }
  // Optionally include search/hash as needed
}, [isErrorUser, navigate, location.pathname, setUser /*, userError*/]);

While here, update the dependency array of the user-sync effect to include setUser (stable but satisfies lint):

useEffect(() => {
  if (user) setUser(user);
}, [user, setUser]);
```<!-- review_comment_end -->

</blockquote></details>
<details>
<summary>src/components/chat/messages/user-message.tsx (2)</summary><blockquote>

`136-161`: **Accessibility: Make document chip and remove button keyboard- and screen reader-accessible**

Same issue previously flagged: clickable Badge without role/tabIndex and icon-only remove without label.



```diff
-            <Badge
+            <Badge
               key={doc._id}
               variant="default"
-              className="group/document flex gap-1.5 py-1 cursor-pointer bg-accent/65 hover:bg-accent/100 hover:text-accent-foreground text-accent-foreground/90 rounded-md transition duration-300 items-center justify-center"
+              className="group/document flex gap-1.5 py-1 cursor-pointer bg-accent/65 hover:bg-accent/100 hover:text-accent-foreground text-accent-foreground/90 rounded-md transition duration-300 items-center justify-center focus:outline-none focus-visible:ring-2 focus-visible:ring-offset-2"
+              role="button"
+              tabIndex={0}
+              aria-label={`Preview document ${doc.name}`}
               onClick={() => handlePreview(doc._id)}
+              onKeyDown={(e) => {
+                if (e.key === "Enter" || e.key === " ") {
+                  e.preventDefault();
+                  handlePreview(doc._id);
+                }
+              }}
             >
               <div className="relative h-4 w-4">
                 <Icon
-                  className={`${IconClassName} h-4 w-4 group-hover/document:opacity-0 transition duration-300`}
+                  className={`${IconClassName} h-4 w-4 group-hover/document:opacity-0 group-focus-within/document:opacity-0 transition duration-300`}
                 />
                 <Button
                   variant="ghost"
                   size="icon"
-                  className="absolute inset-0 h-4 w-4 opacity-0 group-hover/document:opacity-100 hover:bg-muted-foreground/20 cursor-pointer transition duration-300"
+                  className="absolute inset-0 h-4 w-4 opacity-0 group-hover/document:opacity-100 group-focus-within/document:opacity-100 focus:opacity-100 hover:bg-muted-foreground/20 cursor-pointer transition duration-300"
+                  aria-label={`Remove document ${doc.name}`}
+                  title="Remove"
                   onClick={(e) => {
                     e.stopPropagation();
                     onRemove(doc._id);
                   }}
                 >
                   <XIcon className="w-3 h-3" />
                 </Button>
               </div>

---

265-265: Fix layout regression: min-w-[100vw] causes overflow

Replace with responsive width that respects parent constraints.

-              className="bg-transparent resize-none border-none min-w-[100vw] max-w-full ring-0 focus-visible:ring-0 focus-visible:ring-offset-0 outline-none focus-visible:outline-none text-base"
+              className="bg-transparent resize-none border-none w-full min-w-0 max-w-full ring-0 focus-visible:ring-0 focus-visible:ring-offset-0 outline-none focus-visible:outline-none text-base"

src/components/topnav.tsx (3)

14-19: Remove <Navigate /> import if switching to imperative navigation

If you move the auth-redirect into an effect (recommended below), <Navigate /> becomes unused. Drop it to prevent lint failures.

 import {
-  Navigate,
   useLocation,
   useNavigate,
   useParams,
 } from "@tanstack/react-router";

---

36-40: Gate auth redirect on Unauthorized errors; also capture error from the query

Right now every fetch error redirects to /auth. That can misroute on transient/network errors. Capture the error object so you can conditionally redirect only for 401/Unauthorized. Consider also tuning retry behavior.

Apply within this block:

   const {
     data: user,
     isError: isErrorUser,
+    error: userError,
     isLoading: isLoadingUser,
   } = useQuery({
     ...convexQuery(api.auth.getUser, {}),
   });

Then, in an effect (see separate comment), redirect only when userError indicates Unauthorized. Please confirm the error shape from convex + tanstack-query in your setup (e.g., HTTP status, code, or message) before gating. If Unauthorized isn’t distinguishable, at least set retry: false on this query to avoid thrashing on auth failures.

Verification suggestions:

• Check what userError looks like for an expired session vs. a network outage.
• If it exposes an HTTP status, gate on 401 and keep the TopNav rendered otherwise (you could show an inline error or nothing).

If you want, I can wire the conditional retry function once you confirm the error shape.

---

102-115: Add an aria-label to the Settings button for better a11y

ModeToggle already has an sr-only label; mirror that for Settings to improve screen reader support.

         {!resizePanelOpen ? (
           <Button
             variant="ghost"
             size="icon"
             className="cursor-pointer"
+            aria-label="Open settings"
             onClick={() => {
               navigate({ to: "/settings/profile" });
             }}
           >
             <Settings2Icon className="h-6 w-6" />
           </Button>
         ) : null}

src/hooks/chats/use-documents.ts (2)

35-41: Add onError handling for the mutation to provide user feedback

Silent failures here can leave UI state stale. Add onError to surface issues to the user.

-      updateChatInputMutation({
-        chatId: chatId,
-        updates: {
-          documents: filteredDocuments,
-        },
-      });
+      updateChatInputMutation(
+        {
+          chatId,
+          updates: {
+            documents: filteredDocuments,
+          },
+        },
+        {
+          onError: (err) => {
+            toast("Failed to remove document", {
+              description: String(err),
+            });
+          },
+        },
+      );

---

73-96: Optional: parallelize uploads with bounded concurrency

Uploads are fully sequential; for many files this increases latency. Consider parallelizing with a small concurrency limit (e.g., 3–5) to reduce total time while avoiding server overload.

If you want, I can provide a throttled Promise pool snippet tailored to this hook.

Also applies to: 98-108

src/components/chat/input/document-list.tsx (1)

45-66: Accessibility: Make badge keyboard-focusable and label the remove button

The badge is clickable but not keyboard accessible; the icon-only remove button lacks an aria-label and isn’t focus-visible.

-      <Badge
-        variant="default"
-        className="group flex gap-1.5 py-1 cursor-pointer bg-accent/65 hover:bg-accent/100 hover:text-accent-foreground text-accent-foreground/90 rounded-md transition duration-300 items-center justify-center"
-        onClick={handlePreview}
-      >
+      <Badge
+        variant="default"
+        className="group/document flex gap-1.5 py-1 cursor-pointer bg-accent/65 hover:bg-accent/100 hover:text-accent-foreground text-accent-foreground/90 rounded-md transition duration-300 items-center justify-center focus:outline-none focus-visible:ring-2 focus-visible:ring-offset-2"
+        role="button"
+        tabIndex={0}
+        aria-label={`Preview document ${doc.name}`}
+        onClick={handlePreview}
+        onKeyDown={(e) => {
+          if (e.key === "Enter" || e.key === " ") {
+            e.preventDefault();
+            handlePreview();
+          }
+        }}
+      >
         <div className="relative h-4 w-4">
           <Icon
-            className={`${IconClassName} h-4 w-4 group-hover:opacity-0 transition duration-300`}
+            className={`${IconClassName} h-4 w-4 group-hover/document:opacity-0 group-focus-within/document:opacity-0 transition duration-300`}
           />
           <Button
             variant="ghost"
             size="icon"
-            className="absolute inset-0 h-4 w-4 opacity-0 group-hover:opacity-100 hover:bg-muted-foreground/20 cursor-pointer transition duration-300"
+            className="absolute inset-0 h-4 w-4 opacity-0 group-hover/document:opacity-100 group-focus-within/document:opacity-100 focus:opacity-100 hover:bg-muted-foreground/20 cursor-pointer transition duration-300"
+            aria-label={`Remove document ${doc.name}`}
+            title="Remove"
             onClick={handleRemove}
           >
             <XIcon className="w-3 h-3" />
           </Button>
         </div>

Also applies to: 50-61

src/components/document-dialog.tsx (2)

49-91: Optional: guard against setting state after unmount or document switch

The async preview loader can update state after the dialog closes or the doc changes. Add a cancellation flag.

   useEffect(() => {
-    setPreviewUrl(null);
+    let cancelled = false;
+    setPreviewUrl(null);
     const loadPreviewUrl = async () => {
       if (!document) return;
       try {
         switch (tag) {
@@
-            setPreviewUrl(url);
+            if (!cancelled) setPreviewUrl(url);
             break;
           }
@@
-            setPreviewUrl(document.key as string);
+            if (!cancelled) setPreviewUrl(document.key as string);
             break;
           }
@@
-            setPreviewUrl(`https://www.youtube.com/embed/${document.key}`);
+            if (!cancelled)
+              setPreviewUrl(`https://www.youtube.com/embed/${document.key}`);
             break;
           }
           default:
             if (["file", "text", "github"].includes(document.type)) {
               const url = await generateDownloadUrl({
                 documentId: document._id!,
               });
-              setPreviewUrl(url);
+              if (!cancelled) setPreviewUrl(url);
               break;
             } else {
-              setPreviewUrl(document.key as string);
+              if (!cancelled) setPreviewUrl(document.key as string);
             }
             break;
         }
       } catch (e) {
-        setPreviewUrl(null);
+        if (!cancelled) setPreviewUrl(null);
       }
     };
     loadPreviewUrl();
-  }, [document, tag, generateDownloadUrl]);
+    return () => {
+      cancelled = true;
+    };
+  }, [document, tag, generateDownloadUrl]);

---

122-127: Nit: use sizeClassName for spinner sizing for consistency

LoadingSpinner supports sizeClassName specifically for icon sizing.

-          <LoadingSpinner className="h-8 w-8" />
+          <LoadingSpinner sizeClassName="h-8 w-8" />

src/components/chat/messages/user-message.tsx (1)

110-118: Fix typo: double space in error title

Minor polish.

-        <ErrorState
-          title="Error loading attached  documents"
-          error={documentsError}
-        />
+        <ErrorState title="Error loading attached documents" error={documentsError} />

src/components/app-sidebar/index.tsx (2)

79-86: Optional: prevent repeated loadMore triggers

IntersectionObserver will call loadMore each time the sentinel re-enters view. Unobserve after the first trigger and re-observe once status flips back to "CanLoadMore".

     const observer = new IntersectionObserver(
       (entries) => {
         const [entry] = entries;
         if (entry.isIntersecting) {
-          loadMore(15);
+          observer.unobserve(loadMoreElement);
+          loadMore(15);
         }
       },

---

155-163: Nit: add accessible label to the clear-search button

Helps screen readers discover the control.

               <Button
                 variant="ghost"
                 size="icon"
                 onClick={() => setSearchQuery("")}
                 className="flex items-center justify-center size-5"
                 tabIndex={-1}
+                aria-label="Clear search"
+                title="Clear"
               >

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: CR
PR: 0bs-chat/zerobs#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-01T07:34:38.459Z
Learning: Applies to convex/**/*.{ts,js} : Use the api object from convex/_generated/api.ts to call public functions and the internal object to call internal functions.

src/components/app-sidebar/index.tsx (1)

233-239: Fix unreachable “No matching chats found” message (wrong condition).

The empty-state for search results is guarded by !isSearching, so “No matching chats found” will never render during a search. This contradicts both the intent and the AI summary, and it suppresses useful feedback to users.

Apply this diff to show:

• “No matching chats found” only when searching, not loading, not error, and both pinned and history search lists are empty.
• “No chat history” only when not searching and history is empty.

-                {displayHistoryChats.length === 0 && !isSearching && (
-                  <div className="text-sm text-muted-foreground text-center py-2">
-                    {isSearching
-                      ? "No matching chats found"
-                      : "No chat history"}
-                  </div>
-                )}
+                {isSearching &&
+                  !isLoadingSearch &&
+                  !isSearchError &&
+                  displayHistoryChats.length === 0 &&
+                  displayPinnedChats.length === 0 && (
+                  <div className="text-sm text-muted-foreground text-center py-2">
+                    No matching chats found
+                  </div>
+                )}
+                {!isSearching && displayHistoryChats.length === 0 && (
+                  <div className="text-sm text-muted-foreground text-center py-2">
+                    No chat history
+                  </div>
+                )}

src/components/app-sidebar/index.tsx (6)

141-154: Add accessible name to the search input and drop redundant inline style.

Improve a11y by providing an explicit accessible name; the inline transparent background is redundant with the Tailwind classes.

             <Input
               placeholder="Search chats"
               className="flex-1 bg-transparent border-none focus:ring-0 focus:ring-offset-0 focus-visible:ring-0 focus-visible:ring-offset-0 outline-none text-base px-0"
               type="text"
+              aria-label="Search chats"
               value={searchQuery}
               onChange={(e) => setSearchQuery(e.target.value)}
               onKeyDown={(e) => {
                 if (e.key === "Escape") {
                   setSearchQuery("");
                   e.currentTarget.blur();
                 }
               }}
-              style={{ backgroundColor: "transparent" }}
             />

---

156-164: Add accessible name to the clear-search button.

Icon-only controls need an accessible label for screen readers.

               <Button
                 variant="ghost"
                 size="icon"
                 onClick={() => setSearchQuery("")}
                 className="flex items-center justify-center size-5"
                 tabIndex={-1}
+                aria-label="Clear search"
+                title="Clear search"
               >
                 <XIcon className="w-4 h-4" />
               </Button>

---

181-188: AccordionTrigger without children: add an accessible label (or make the whole header the trigger).

AccordionTrigger is rendered without children, so the control has no accessible name and only the chevron is clickable. At minimum, add an aria-label. Alternatively, consider making the entire header (icon + “Pinned”) the trigger for a larger tap target.

-                  <AccordionTrigger className="cursor-pointer p-0 mr-2" />
+                  <AccordionTrigger
+                    className="cursor-pointer p-0 mr-2"
+                    aria-label="Toggle pinned chats"
+                  />

---

171-176: Avoid mixing controlled and uncontrolled Accordion props.

value makes the Accordion controlled; defaultValue is ignored and can be removed for clarity.

             <Accordion
               type="single"
               collapsible
-              defaultValue="pinned"
               className="w-full"
               value={pinnedChatsAccordionOpen ? "pinned" : ""}

---

84-84: Replace magic number 15 with a named constant.

Removes duplication, clarifies intent, and centralizes changes to page size.

-          loadMore(15);
+          loadMore(PAGE_SIZE);

-                    onClick={() => loadMore(15)}
+                    onClick={() => loadMore(PAGE_SIZE)}

Add this near the top of the module (e.g., after imports):

const PAGE_SIZE = 15;

Also applies to: 228-228

---

218-221: Optionally surface error details to users.

Consider displaying the underlying error message (e.g., searchError?.message) in ErrorState to aid troubleshooting. If you adopt this, also destructure searchError from useSearchChats() above.

Would you like me to wire this up and ensure the ErrorState component’s API is respected?

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

src/routes/__root.tsx (1)

79-87: Good fix: moved layout state updates out of render and into an effect.

This resolves the earlier “state mutation during render” issue for settings routes and prevents render loops. Nice job.

src/components/topnav.tsx (1)

18-23: Fix conditional hook usage caused by early return.

Returning before subsequent hooks (e.g., useEffect below) violates React’s rules of hooks. Navigate via an effect instead and keep all hooks unconditional. This also addresses the same concern noted previously.

Apply these diffs:

@@
-import {
-  Navigate,
-  useLocation,
-  useNavigate,
-  useParams,
-} from "@tanstack/react-router";
+import { useLocation, useNavigate, useParams } from "@tanstack/react-router";
@@
-  if (isErrorUser) {
-    return <Navigate to="/auth" />;
-  }
+  useEffect(() => {
+    if (isErrorUser) {
+      navigate({ to: "/auth" });
+    }
+  }, [isErrorUser, navigate]);

#!/bin/bash
# Audit for other early <Navigate /> returns that may conditionally skip hooks.
# Shows surrounding context to verify ordering.
rg -nP -C3 --type=tsx '\breturn\s*<Navigate\b'

Also applies to: 49-51

src/components/ui/accordion.tsx (6)

108-114: Broaden Trigger props to accept native button attributes (disabled, aria-*, id).

You already spread ...props onto a button but the type doesn’t allow native attributes, which can break consumers (e.g., disabled). Extend from the intrinsic button props.

Apply this diff:

-interface AccordionTriggerProps {
-  className?: string;
-  children?: React.ReactNode;
-  onClick?: React.MouseEventHandler<HTMLButtonElement>;
-  showIcon?: boolean;
-}
+interface AccordionTriggerProps
+  extends React.ButtonHTMLAttributes<HTMLButtonElement> {
+  showIcon?: boolean;
+}

---

29-37: Let Accordion pass through div attributes safely.

AccordionProps doesn’t extend HTML div attributes but you spread ...props to a div. Extend from the intrinsic div props to avoid type holes and enable id/aria/class usage.

Apply this diff:

-interface AccordionProps {
+interface AccordionProps extends React.HTMLAttributes<HTMLDivElement> {
   type?: "single" | "multiple";
   collapsible?: boolean;
   value?: string | string[];
   defaultValue?: string | string[];
   onValueChange?: (value: string | string[]) => void;
-  className?: string;
-  children?: React.ReactNode;
+  className?: string; // optional: redundant, but keep explicit if you prefer
+  children?: React.ReactNode; // ditto
 }

---

102-107: Same pass-through typing for AccordionItem.

AccordionItemEnhanced forwards ...props to a div. Let AccordionItemProps extend HTML div attributes for correctness and flexibility.

Apply this diff:

-interface AccordionItemProps {
+interface AccordionItemProps extends React.HTMLAttributes<HTMLDivElement> {
   value: string;
   className?: string;
   children?: React.ReactNode;
 }

---

163-167: Same pass-through typing for AccordionContent.

AccordionContent spreads ...props to the animated div; extend from HTML div attributes to type that properly.

Apply this diff:

-interface AccordionContentProps {
+interface AccordionContentProps extends React.HTMLAttributes<HTMLDivElement> {
   className?: string;
   children?: React.ReactNode;
 }

---

150-151: Chevron rotation direction—confirm intended UX.

Currently the chevron points down when open and up when closed (isOpen ? 0 : 180). Many designs do the opposite. If you want “rotate up when open,” flip the values.

Apply this diff if that’s the intended behavior:

-            animate={{ rotate: isOpen ? 0 : 180 }}
+            animate={{ rotate: isOpen ? 180 : 0 }}

---

153-153: Trim trailing whitespace in className.

Minor cleanup; there’s a trailing space at the end of the className value.

Apply this diff:

-            className="items-center flex justify-center "
+            className="items-center flex justify-center"

src/routes/__root.tsx (3)

110-116: Use the next open value from onOpenChange instead of toggling via captured state.

Relying on the captured sidebarOpen can desync state if multiple updates fire quickly. Prefer the callback’s next open value.

Apply this diff:

-          <SidebarProvider
+          <SidebarProvider
             className="font-sans h-svh relative before:content-[''] before:fixed before:inset-0 before:bg-[url('/images/noise.png')] before:opacity-50 before:pointer-events-none before:z-[-1]"
             open={sidebarOpen}
-            onOpenChange={() => {
-              setSidebarOpen(!sidebarOpen);
-            }}
+            onOpenChange={(open) => {
+              setSidebarOpen(open);
+            }}
           >

---

117-123: Centralize settings-route layout decisions (optional).

TopNav hides itself via CSS on settings while Root always renders it. Consider conditionally rendering TopNav here (like AppSidebar) to avoid mounting work on settings pages and to keep route-specific layout logic in one place.

Potential adjustment:

-                {!isSettingsRoute && <AppSidebar />}
-                <TopNav />
+                {!isSettingsRoute && <AppSidebar />}
+                {!isSettingsRoute && <TopNav />}

---

74-78: Nit: hoist static publicRoutes outside the component.

This avoids re-allocating the array every render and makes intent clearer. Also consider broadening the check if you introduce nested auth routes (e.g., startsWith("/auth")).

Would you like me to send a follow-up patch that hoists the constant and, if needed, adapts the check for nested auth paths?

src/components/topnav.tsx (4)

53-57: Add setUser to effect deps.

The setter is stable, but including it clarifies intent and silences strict-lint configurations.

-  }, [user]);
+  }, [user, setUser]);

---

41-47: Auth error handling nuance (optional).

Redirecting on any query error may bounce users to /auth on transient network failures. If possible, gate on an “unauthorized”/auth-specific error code, otherwise surface a non-blocking ErrorState.

Want me to update this to inspect error types from convex/react-query (e.g., unauthorized vs network) and only redirect on auth failures?

---

113-125: Accessibility: add an aria-label to the Settings button.

Screen reader users won’t get a name from the icon alone.

-          <Button
+          <Button
             variant="ghost"
             size="icon"
             className="cursor-pointer"
             onClick={() => {
               navigate({ to: "/settings/profile" });
             }}
+            aria-label="Open settings"
           >
             <Settings2Icon className="h-6 w-6" />
           </Button>

---

85-88: Optional: avoid duplicate settings-route visibility logic.

Root decides sidebar visibility for settings routes; TopNav also computes and hides itself. Consider relying on the root to conditionally render TopNav (or lifting a shared helper) to keep this concern in one place.

src/components/chat/input/document-list.tsx (5)

45-49: Improve a11y: title on Badge, aria-label on Remove, and keyboard-visible state.

Add accessible labels and ensure the remove button becomes visible on keyboard focus, not only on hover.

@@
-      <Badge
+      <Badge
         variant="default"
         className="group/badge flex  gap-1.5 py-1.5 cursor-pointer bg-accent/65 hover:bg-accent/100 hover:text-accent-foreground text-accent-foreground/90  transition duration-300 items-center justify-center"
+        title={doc.name}
         onClick={handlePreview}
       >
@@
-          <Button
+          <Button
             variant="ghost"
             size="icon"
-            className="absolute inset-0 h-4 w-4 opacity-0 group-hover/badge:opacity-100 hover:bg-muted-foreground/20 cursor-pointer transition duration-300"
+            aria-label={`Remove document ${doc.name}`}
+            title={`Remove document ${doc.name}`}
+            className="absolute inset-0 h-4 w-4 opacity-0 group-hover/badge:opacity-100 group-focus-within/badge:opacity-100 focus-visible:opacity-100 hover:bg-muted-foreground/20 cursor-pointer transition duration-300 focus-visible:ring-1 focus-visible:ring-ring focus-visible:outline-none"
             onClick={handleRemove}
           >
-            <XIcon className="w-3 h-3" />
+            <XIcon className="w-3 h-3" aria-hidden="true" />
           </Button>

Also applies to: 54-61, 60-60

---

115-118: Minor: Use LoadingSpinner’s label prop instead of a separate text node.

Keeps semantics consistent and reduces DOM.

-          <LoadingSpinner sizeClassName="h-4 w-4" />
-          Loading attached documents...
+          <LoadingSpinner sizeClassName="h-4 w-4" label="Loading attached documents..." />

---

123-134: Simplify error condition and optionally surface the message.

isError already reflects presence of error. Consider relying on it and optionally pass a compact description.

-  if (isDocumentsError || documentsError) {
+  if (isDocumentsError) {
     return (
       <div className="flex items-center justify-start p-2">
         <ErrorState
           title="Error loading documents"
-          showDescription={false}
+          description={documentsError instanceof Error ? documentsError.message : undefined}
+          showDescription={Boolean(documentsError)}
           className="p-2"
           density="compact"
         />
       </div>
     );
   }

---

12-13: Optional: centralize fetching via a useDocuments hook for consistency.

For parity with the rest of the codebase and to avoid repeating query wiring, consider replacing the inline useQuery block with a dedicated hook (e.g., from @/hooks/chats/use-documents) that returns { documents, isLoadingDocuments, isDocumentsError, documentsError }.

-import { useRemoveDocument } from "@/hooks/chats/use-documents";
+import { useRemoveDocument, useDocuments } from "@/hooks/chats/use-documents";
@@
-  } = useQuery({
-    ...convexQuery(api.documents.queries.getMultiple, { documentIds }),
-    enabled: documentIds.length > 0,
-  });
+  } = useDocuments(documentIds);

Note: If you adopt this, retain the post-error empty-docs guard added in the previous comment.

Also applies to: 79-86

---

14-14: Tiny micro-optimization: memoize modalities lookup.

Not critical, but avoids re-running find on each render.

-import React, { useCallback } from "react";
+import React, { useCallback, useMemo } from "react";
@@
-  const selectedModel = models.find((m) => m.model_name === model);
-  const modalities = selectedModel?.modalities;
+  const modalities = useMemo(
+    () => models.find((m) => m.model_name === model)?.modalities,
+    [model]
+  );

Also applies to: 107-109

src/components/chat/messages/index.tsx (4)

15-16: Destructured hook return looks good; consider clearer naming.

The expanded shape matches the updated hook. Consider renaming error to messagesError locally to avoid confusion with streamError in the same scope.

-  const { isLoading, isEmpty, isError, error, isStreamError, streamError } =
+  const { isLoading, isEmpty, isError, error: messagesError, isStreamError, streamError } =
     useMessages({ chatId });

And update usages accordingly (e.g., error={messagesError}).

---

43-46: Use LoadingSpinner’s label prop to avoid duplicate, misaligned text.

Slight simplification and better a11y by keeping the label tied to the spinner component.

-        <div className="flex items-center justify-center h-full">
-          <LoadingSpinner />
-          <div className="text-muted-foreground">Loading messages...</div>
-        </div>
+        <div className="flex items-center justify-center h-full">
+          <LoadingSpinner label="Loading messages..." />
+        </div>

---

50-62: Guard general error branch on isError only to prevent stale-object flicker.

React Query ensures error is meaningful only when isError is true. Checking error alone can render the error state during refetches or intermediate states if a stale error object lingers.

-    if (isError || error) {
+    if (isError) {
       return (
         <div className="flex items-center justify-center w-full h-full">
           <ErrorState
             className="max-w-4xl"
             title="Failed to load messages"
-            error={error}
+            error={error}
             description="Please try again later."
             density="comfy"
           />
         </div>
       );
     }

---

64-77: Similarly, gate streaming error UI on isStreamError only.

streamError should be treated as payload; the boolean flag should control the branch to avoid false positives if a stale value exists.

-    if (isStreamError || streamError) {
+    if (isStreamError) {
       return (
         <div className="flex items-center justify-center w-full h-full">
           <ErrorState
             className="max-w-4xl"
             density="comfy"
             description="Unable to load messages, this might be due to either a network issue or a server error."
             title="Error loading messages"
             error={streamError}
             showIcon={false}
           />
         </div>
       );
     }

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: CR
PR: 0bs-chat/zerobs#0
File: .cursor/rules/convex_rules.mdc:0-0
Timestamp: 2025-08-01T07:34:38.459Z
Learning: Applies to convex/**/*.{ts,js} : Use the api object from convex/_generated/api.ts to call public functions and the internal object to call internal functions.

src/components/ui/accordion.tsx (1)

115-122: Prop reordering is safe; default remains explicit.

Moving showIcon to the front with showIcon = true keeps behavior clear and non-breaking for named props. No issues spotted.

src/routes/__root.tsx (2)

9-9: Spinner migration looks good.

Adopting the shared LoadingSpinner here aligns the route’s loading UI with the rest of the app.

---

92-92: Minor UI: spinner sizing reads well against full-screen container.

The sizeClassName choice keeps the loading state visually balanced. No action needed.

src/components/topnav.tsx (1)

107-111: Nice: lightweight loading affordance in the control cluster.

The compact LoadingSpinner integrates cleanly without shifting layout. No changes needed.

src/components/chat/input/document-list.tsx (2)

7-8: LGTM: central loading/error UI adoption.

Good use of the shared LoadingSpinner and ErrorState components for consistent UX.

---

85-86: Good: follows Convex rule to use generated api object.

Query is properly issued via api.documents.queries.getMultiple. Matches the team guideline in retrieved learnings.

src/components/chat/messages/index.tsx (2)

2-2: Nice addition: centralizing error UI via ErrorState.

Import looks correct and aligns with the PR goal of consistent error handling across components.

---

11-11: Loading spinner integration is consistent with shared UI.

Good reuse of the shared LoadingSpinner component.

src/components/topnav.tsx (1)

47-49: Early return with breaks hook order — move redirect into an effect

This conditional return causes hooks below (useEffect/useParams) to be skipped on error renders, violating the Rules of Hooks (Biome errors at Lines 51, 58, 64). Redirect via useEffect instead. This also mirrors the earlier review feedback.

Apply:

-  if (isErrorUser) {
-    return <Navigate to="/auth" />;
-  }
+  // Redirect to auth on user fetch error without breaking hooks order
+  useEffect(() => {
+    if (isErrorUser) {
+      navigate({ to: "/auth" });
+    }
+  }, [isErrorUser, navigate]);

src/components/topnav.tsx (6)

18-23: Remove unused Navigate import after redirect refactor

Once the early return is removed, Navigate is no longer needed.

-import {
-  Navigate,
-  useLocation,
-  useNavigate,
-  useParams,
-} from "@tanstack/react-router";
+import { useLocation, useNavigate, useParams } from "@tanstack/react-router";

---

39-45: Harden auth query behavior to avoid thrash and unnecessary refetch

Auth failures can loop retries and trigger repeated redirects. Consider turning off retries and giving the user doc a short cache window.

-  } = useQuery({
-    ...convexQuery(api.auth.getUser, {}),
-  });
+  } = useQuery({
+    ...convexQuery(api.auth.getUser, {}),
+    retry: false,
+    staleTime: 60_000,
+  });

---

63-81: Guard global Cmd/Ctrl+I against text inputs/contentEditable

Prevents accidental panel toggles while typing in inputs or rich editors (Cmd/Ctrl+I is a common “italic” shortcut).

   const handleKeyDown = (event: KeyboardEvent) => {
-    if (
+    const target = event.target as HTMLElement | null;
+    if (target && (target.isContentEditable || ["INPUT", "TEXTAREA"].includes(target.tagName))) {
+      return;
+    }
+    if (
       !event.repeat &&
       event.key === "i" &&
       (event.metaKey || event.ctrlKey)
     ) {

---

120-129: Use functional updater to avoid stale state on atom toggle

Minor, but the functional form is safer and consistent with your keybinding handler.

-            onClick={() => {
-              setResizePanelOpen(!resizePanelOpen);
-              setSelectedArtifact(undefined);
-            }}
+            onClick={() => {
+              setResizePanelOpen(prev => !prev);
+              setSelectedArtifact(undefined);
+            }}

---

119-136: UX: Consider disabling (not hiding) the panel toggle when an artifact/MCP is selected

Hiding controls shifts layout and removes affordance; disabling with a tooltip preserves layout and sets user expectation.

Example (conceptual):

- {isOnChatRoute && (
+ {isOnChatRoute && (
     <Button
       variant="ghost"
       size="icon"
-      className={`${selectedArtifact || selectedVibzMcp ? "hidden" : ""}`}
+      className=""
+      disabled={!!(selectedArtifact || selectedVibzMcp)}
       aria-label="Toggle right panel"

---

83-139: Optional: Avoid nav flicker while user is loading

If you notice a brief topbar flash before redirect, you can hide the bar during initial auth resolution without breaking hook order (return occurs after all hooks).

 export function TopNav() {
   ...
-  return (
+  if (isLoadingUser) return null;
+  return (
     <div

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

src/components/document-dialog.tsx (7)

24-34: Confirm conditional query behavior; consider adding enabled to avoid accidental fetches

You rely on convexQuery(..., "skip"), which is fine if the wrapper handles the sentinel. To be safe with TanStack Query semantics and avoid any accidental fetches when the dialog is closed, add an explicit enabled flag keyed to documentDialogOpen.

   } = useQuery({
     ...convexQuery(
       api.documents.queries.get,
       documentDialogOpen ? { documentId: documentDialogOpen } : "skip",
     ),
+    enabled: !!documentDialogOpen,
   });

If convexQuery already guarantees no-op when passed "skip", this remains a no-op safety net. Please verify current wrapper behavior.

---

97-105: Broaden download support (image/pdf/audio/video) and guard with try/catch

Users often want to download images/PDFs/media too. Also add basic error handling around the mutation.

 const handleDownload = async () => {
-  if (!document || tag !== "file") return;
-  const url = await generateDownloadUrl({
-    documentId: document._id!,
-  });
-  if (url) {
-    window.open(url, "_blank");
-  }
+  if (!document) return;
+  // Skip external-only resources
+  if (tag === "url" || tag === "site" || tag === "youtube") return;
+  try {
+    const url = await generateDownloadUrl({
+      documentId: document._id!,
+    });
+    if (url) {
+      const w = window.open(url, "_blank", "noopener,noreferrer");
+      if (w) w.opener = null;
+    }
+  } catch (e) {
+    console.error("Failed to generate download url", e);
+  }
 };
 
 ...
 
-          {tag === "file" ? (
+          {(tag === "file" || tag === "pdf" || tag === "image" || tag === "audio" || tag === "video") ? (
             <Button onClick={handleDownload}>Download</Button>
           ) : tag === "url" || tag === "site" || tag === "youtube" ? (
             <Button onClick={handleOpen}>
               Open {tag === "youtube" ? "in YouTube" : "in Browser"}
             </Button>
           ) : null}
```<!-- review_comment_end -->



Also applies to: 261-267

---

`172-176`: **Show “0 Bytes” correctly when size is zero**

The current truthy check hides size 0. Use a null/undefined check instead.


```diff
-          {document?.size && (
+          {document?.size != null && (
             <div className="text-sm text-muted-foreground">
               Size: {formatBytes(document.size)}
             </div>
           )}
```<!-- review_comment_end -->

---

`202-212`: **Add title for PDF object for a11y**

Mirror the file fallback’s title and improve accessibility.


```diff
           <object
+            title="PDF preview"
             data={previewUrl}
             className="absolute inset-0 w-full h-full"
             type="application/pdf"
           >
```<!-- review_comment_end -->

---

`224-237`: **Revisit iframe sandbox policy for external sites**

Sandboxing with allow-same-origin + allow-scripts reduces isolation. If the content is untrusted and you don’t need DOM access, consider dropping allow-same-origin for stronger origin isolation. If you want links to work, add allow-popups (and optionally allow-top-navigation-by-user-activation). You may also want to add referrerPolicy="no-referrer".



Example:

```tsx
<iframe
  title="Website preview"
  src={previewUrl}
  className="absolute inset-0 w-full h-full"
  sandbox="allow-scripts allow-popups allow-top-navigation-by-user-activation"
  referrerPolicy="no-referrer"
/>

Please verify UX expectations (do links inside the frame need to open?) and adjust accordingly.

---

40-47: Minor naming consistency: use lowerCamelCase for iconClassName

Renaming improves consistency with typical React prop naming.

-  const {
-    icon: Icon,
-    className: IconClassName,
-    tag,
-  } = document
+  const {
+    icon: Icon,
+    className: iconClassName,
+    tag,
+  } = document
     ? getDocTagInfo(document)
     : { icon: () => null, className: "", tag: "" };

-            <Icon className={`${IconClassName} h-8 w-8`} />
+            <Icon className={`${iconClassName} h-8 w-8`} />
```<!-- review_comment_end -->



Also applies to: 160-161

---

`189-257`: **Optional: show a lightweight spinner while generating preview URLs**

Between document load and previewUrl resolution (signed URL generation), the preview area is blank. Surface a small inline spinner for better perceived performance.


```diff
-  const { mutateAsync: generateDownloadUrl } = useMutation({
+  const { mutateAsync: generateDownloadUrl, isPending: isGeneratingPreview } = useMutation({
     mutationFn: useConvexMutation(api.documents.mutations.generateDownloadUrl),
   });

-          {previewUrl && (
+          {previewUrl ? (
             <div className="flex-grow relative min-h-0 bg-muted rounded-md border overflow-hidden">
               {(() => {
                 if (tag === "image") {
                   return (
                     <img
                       src={previewUrl}
                       alt={documentName}
                       className="absolute inset-0 w-full h-full object-contain"
                     />
                   );
                 } else if (tag === "pdf") {
                   return (
                     <object
                       data={previewUrl}
                       className="absolute inset-0 w-full h-full"
                       type="application/pdf"
                     >
                       <div className="absolute inset-0 flex items-center justify-center text-muted-foreground">
                         PDF preview not supported in your browser. Please
                         download the file to view it.
                       </div>
                     </object>
                   );
                 } else if (tag === "youtube") {
                   return (
                     <iframe
                       title="YouTube video player"
                       src={previewUrl}
                       className="absolute inset-0 w-full h-full"
                       allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture"
                       allowFullScreen
                     />
                   );
                 } else if (tag === "url" || tag === "site") {
                   return (
                     <iframe
                       title="Website preview"
                       src={previewUrl}
                       className="absolute inset-0 w-full h-full"
                       sandbox="allow-same-origin allow-scripts"
                       style={{
                         transform: "scale(0.95)",
                         transformOrigin: "top left",
                         width: "105.3%", // 100/0.95 to compensate for scale
                         height: "105.3%",
                       }}
                     />
                   );
                 } else if (tag === "file") {
                   // fallback for unknown file types
                   return (
                     <object
                       title="File preview"
                       data={previewUrl}
                       className="absolute inset-0 w-full h-full"
                       type="application/octet-stream"
                     >
                       <div className="absolute inset-0 flex items-center justify-center text-muted-foreground">
                         File preview not supported. Please download the file to
                         view it.
                       </div>
                     </object>
                   );
                 }
                 return null;
               })()}
             </div>
-          )}
+          ) : !previewError ? (
+            <div className="flex-grow flex items-center justify-center bg-muted rounded-md border">
+              <LoadingSpinner className="h-5 w-5" label="Preparing preview..." />
+            </div>
+          ) : null}
```<!-- review_comment_end -->
<!-- file_end -->


Also applies to: 36-38

</blockquote></details>
<details>
<summary>src/components/topnav.tsx (4)</summary><blockquote>

`31-36`: **Also wire a setter for `selectedVibzMcp` (to clear it when opening the right panel)**

Hotkey and button clear `selectedArtifact` before opening the panel, but not `selectedVibzMcp`. Add its setter for consistency.



```diff
-  const selectedVibzMcp = useAtomValue(selectedVibzMcpAtom);
+  const selectedVibzMcp = useAtomValue(selectedVibzMcpAtom);
+  const setSelectedVibzMcp = useSetAtom(selectedVibzMcpAtom);

---

52-71: Hotkey: unify selection clearing and dependency list

When toggling open via Ctrl/Cmd+I, clear both selectedArtifact and selectedVibzMcp so the panel opens in a clean state. Also include the new setter in the effect deps.

   useEffect(() => {
     if (!isOnChatRoute) return;
     const handleKeyDown = (event: KeyboardEvent) => {
       if (
         !event.repeat &&
         event.key === "i" &&
         (event.metaKey || event.ctrlKey)
       ) {
         event.preventDefault();
         setResizePanelOpen((open) => {
-          if (!open) setSelectedArtifact(undefined);
+          if (!open) {
+            setSelectedArtifact(undefined);
+            setSelectedVibzMcp(undefined);
+          }
           return !open;
         });
       }
     };
     window.addEventListener("keydown", handleKeyDown);
     return () => window.removeEventListener("keydown", handleKeyDown);
-  }, [isOnChatRoute, setResizePanelOpen, setSelectedArtifact]);
+  }, [isOnChatRoute, setResizePanelOpen, setSelectedArtifact, setSelectedVibzMcp]);

---

112-129: Use functional state update and clear both selections when toggling via button

Functional updates prevent stale-closure issues. Clear both selections to mirror the hotkey behavior.

   {isOnChatRoute && (
     <Button
       variant="ghost"
       size="icon"
       className={`${selectedArtifact || selectedVibzMcp ? "hidden" : ""}`}
       aria-label="Toggle right panel"
       onClick={() => {
-        setResizePanelOpen(!resizePanelOpen);
-        setSelectedArtifact(undefined);
+        setResizePanelOpen((prev) => !prev);
+        setSelectedArtifact(undefined);
+        setSelectedVibzMcp(undefined);
       }}
     >

Accessibility nitpick (optional):

• Add aria-pressed={resizePanelOpen} or aria-expanded={resizePanelOpen} and aria-controls if the right panel has an id.

---

77-83: Tame complex className strings with a utility for readability

Long template strings are hard to diff and reason about. Consider using a cn/clsx helper or a variant factory (e.g., cva) to compose conditional classes.

Example (illustrative):

import { cn } from "@/lib/utils";

<div
  className={cn(
    "fixed right-0 z-50 flex w-full items-center justify-between bg-transparent py-2 px-1.5 pointer-events-none text-foreground/70",
    isSettingsRoute && "hidden"
  )}
/>

Also applies to: 97-99